Settings: hand-rolled sidebar chrome in place of the TabView - #199
Settings: hand-rolled sidebar chrome in place of the TabView#199T0mSIlver wants to merge 5 commits into
Conversation
The tabbed Settings window had run out of room: five tabs of dense rows in a 580x420 frame, with no place to say what a pane is for. This replaces the TabView with a two-column layout — a fixed 208pt sidebar of plain buttons and a detail column that leads with the pane's title and a one-line subtitle. Chrome only: every pane renders exactly the content it did before. Hand-rolled rather than NavigationSplitView/.sidebarAdaptable on purpose. Those bring a sidebar-collapse toolbar button that can only be removed with private-API hacks, and their rows surface to accessibility as table cells. Plain AXButton rows keep the window under our control and give the AX drills something stable to press. The Settings scene is kept (openSettings(), Cmd-comma and restorationBehavior(.disabled) all ride on it). Width is fixed at 780 and height stays free: .windowResizability(.contentSize) would resize the window under the pointer on every tab switch. Both AX drills had to move with it: `button "<name>" of toolbar 1 of window 1` no longer exists. The probe is now shared (scripts/lib/ax-probe.swift) and gained --press <identifier> and --scope <identifier>. Scoping closes a real hole rather than being cosmetic: pane content asserts now have to match inside settings.pane.<tab>, so a sidebar row's AXStaticText label can no longer satisfy them.
…ute matched The open question is whether SwiftUI surfaces .accessibilityIdentifier on a container in a plain-styled hierarchy. If it does not, every scoped pane assertion in the drill would fail for a reason that has nothing to do with the pane. The fallback is the window's first AXScrollArea, which keeps the property the scope exists for: the sidebar rows and the pane header sit outside it, so a navigation label still cannot satisfy a pane assertion. The success line names the route it took, so one drill run answers the question.
… pane scope Review follow-ups on the drill's two shape-matched fallbacks. Both of them — an AXButton with this title for --press, the first AXScrollArea for --scope — searched every window the app had open. The enrollment sheet alone contains two scroll views, so a presented sheet could have answered a pane assertion, and any window with a "General" button could have answered a tab press. `--window <title-substring>` restricts the search; both drills pass "Settings". A hint that matches nothing searches everything and says so, so a window that stops carrying its title degrades loudly instead of silently matching nothing. ui-smoke now also dumps the tree once before the per-tab assertions and asserts that the pane scope is reachable at all — either a settings.pane.* AXIdentifier or a scroll area. Losing AX identity was previously six 10s timeouts, each blaming its own needle; it is now one named failure, and the pass line records which of the two routes the run used. capture-readme-assets.sh: the three tab tables are indexed together, so a mismatch would have written one tab's screenshot into another tab's file. Length guard, loud exit.
Manual UI review — CI artifactTested the exact prebuilt artifact with Coverage: all five panes, managed and External URL endpoint states, light/dark appearance, scrolling, and close → reopen. Findings
Screenshots (8)
|
SSH enrollment flow — artifact hand-testI exercised the SSH enrollment UI in the CI-built #199 artifact via UI findings
No SSH config was inserted and no remote command was run. The temporary host registry was restored byte-for-byte and both test tokens were invalidated before these screenshots were uploaded. ScreenshotsRow states: empty → partial → ready
Enrollment sheet and confirmations
Verification, uninstall, and notes
|
# Conflicts: # Sources/localvoxtral/SettingsView.swift
The hand-test on the field Mac (PR #199 review) saw `.behindWindow` render as a flat solid fill in both light and dark appearance — this window cannot vibrate what is behind it, so the standard sidebar look never appeared. `.withinWindow` is the documented fallback and is what actually produces a material here.
|
Merged Conflict: only Also applied the one design fix from the hand-test above (finding 2): the sidebar's Proof: |
|
Final review (opencode/GLM-5.2, post-merge state): no findings above nit level. The merge resolution was explicitly verified — the diff of the merged SettingsView.swift against main's is exactly the sidebar rewrite, with every main-side addition (cmux rows, persistent-forward row, plugin-update panel, enrollment step results, About Build row, AboutSettingsPane signature) present and intact. Six nits, dispositions:
|
The coverage tests compare Sets, so an accidental reorder passed every test while moving rows users have muscle memory for. Pinned here at the stack top, where the final five-pane order lives.
1 participant
What
Replaces the five-tab
TabViewSettings window with a hand-rolled two-columnlayout: a fixed 208pt sidebar of plain buttons, and a detail column that leads
with the pane's title and a one-line subtitle. Chrome only — every pane
renders exactly the content it did before, from the same call sites.
Stacked PR 1 of 2. #201 (
settings-row-idiom) targets this branch and rewritesthe row/group primitives inside the panes — review this one first, and do not
delete this branch until #201 is retargeted (GitHub auto-closes children when a
base branch is deleted).
Why hand-rolled instead of
NavigationSplitView/.sidebarAdaptable: both bringa sidebar-collapse toolbar button that can only be removed with private-API
hacks, and their rows surface to accessibility as table cells. Plain
AXButtonrows keep the window under our control and give the AX drills a stable element
with a stable identifier to press.
Details:
Settingsscene is kept, soopenSettings(), Cmd-comma andrestorationBehavior(.disabled)all keep working.not
.windowResizability(.contentSize): that resizes the window under thepointer on every tab switch. Each pane keeps its own
ScrollView.WindowAccessor(
SettingsWindowChrome), applied onviewDidMoveToWindowand ondidBecomeKey— SwiftUI re-asserts its own titlebar config when the scenerebuilds the window.
NSVisualEffectView.sidebarmaterial,.behindWindow, 28pt topinset clearing the traffic lights, 34pt rows with an r8 selection fill, 22×22
tinted icon tiles, About pinned to the bottom over a
v{version} ({build})footer.
Sources/localvoxtral/Settings/{SettingsSidebarView,SettingsPaneHeader,SettingsWindowChrome}.swift.SettingsTabgainstitle/subtitle/systemImage/tintand the two AXidentifiers. Raw values are unchanged — the scripts key off them.
Script updates (mandatory, not optional polish)
Both AX drills clicked
button "<tab>" of toolbar 1 of window 1, which no longerexists. The probe moved to
scripts/lib/ax-probe.swift(shared byui-smoke.shand
capture-readme-assets.sh) and gained:--press <identifier>— find by AXIdentifier,AXUIElementPerformActionkAXPressAction;--title <text>is accepted as an AXTitle fallback, and theprobe prints which route it used, so the log answers the "does
.accessibilityIdentifiersurface on a.plainButton?" question empirically.--scope <identifier>— assert needles ONLY inside that subtree. This closes areal hole: sidebar row labels are
AXStaticTextand could vacuously satisfy apane assert. Every
assert_tabnow has to match insidesettings.pane.<tab>.--window <title-substring>— both fallbacks match by shape (an AXButtonwith this title; the first AXScrollArea), so they must not wander into another
window: the enrollment sheet alone contains two
ScrollViews. Both drills passSettings. A hint that matches no window searches all of them and says so, soa window that stops carrying its title degrades loudly rather than matching
nothing.
--dump-on-failoutput.ui-smoke.shdumps the tree once before the per-tab assertions and asserts thepane scope is reachable at all (a
settings.pane.*identifier or a scrollarea). Losing AX identity used to be six 10s timeouts each blaming its own
needle; it is now one named failure, and the pass line records which route the
run used.
capture-readme-assets.sh: its three tab tables are indexed together, so alength mismatch would have written one tab's screenshot into another tab's
file. Length guard, loud exit.
No assert was weakened; the pane asserts are strictly stronger than before.
Proof
./scripts/remote-build.shRe-run after the review hardening commit (
da5d653), focused then full:CI
build-teston this branch: pass (4m7s).Build is warning-free for the new/changed files (the only warnings in
.build/last-remote.logare the pre-existingClaudePluginInstallService.swift:226try?one and two inClaudeHookPublisherTests).New test —
Tests/localvoxtralTests/SettingsTabTests.swift(6 cases, part of the2021 above): the primary + meta sidebar arrays cover
SettingsTab.allCasesexactly once with no overlap and no duplicates; every case has a non-empty
title/subtitle/symbol/identifier; subtitles are single-line sentences; and both
identifier schemes are pinned to the literal
settings.tab.<rawValue>/settings.pane.<rawValue>strings the scripts hardcode, as are the raw valuesthemselves.
(runs 30309569624
and 30310237279).
Both stopped before any AX assertion ran, at the drill's pre-launch guard:
That is the runner's GUI session having a localvoxtral instance that would not
quit (it is the owner's personal Mac, ~00:20 local). The drill deliberately
refuses to kill it rather than force-quitting someone's running app, and I did
not weaken that guard to get a green. The AX selectors are therefore verified
by code-reading only — please re-dispatch
ui-smoke.ymlon this branch once theMac is free.
capture-assets.yml(which would have produced screenshots andthe same evidence) was deliberately NOT dispatched: it
pkills the running appand switches the Mac to dark mode, which is not something to do to an owner who
is evidently at the keyboard
two GUI drill scripts. Nothing in the diff touches prompts, model pins,
sampling, the polish request shape, or anything else that reaches the model
(
scripts/ci/llm-lane-filter.shmatches nothing here).UI change — what was verified, and what was not
Verified here: compiles clean under Swift 6.2 strict concurrency; the full unit
suite; the sidebar/pane data model by test; and the drill selectors by
code-reading plus a
deliberate belt-and-braces in the probe:
--pressfalls back to the row's AXTitleand
--scopefalls back to the window's first AXScrollArea, each printing whichroute it took. So the drill answers "does
.accessibilityIdentifiersurface on a.plainButton / on a pane container?" on its first real run without beingable to break on the answer. The fallback keeps the anti-vacuous property: the
sidebar rows and pane header are outside the pane's scroll area.
Not verified — needs the owner's eyes on a real window (
./scripts/try-pr.sh <this PR>):.behindWindowsidebar material vibrates or renders flat in thiswindow. If it is flat, the documented one-line fallback is
.withinWindowinSettingsSidebarBackground.Settings window (the
didBecomeKeyre-apply is there for exactly that). If itever fights the scene irrecoverably, the documented fallback is to keep the
standard titlebar.
scale, and that the 780×560 default is comfortable for the longest pane
(Text Processing).